Skip to content

STYLE use pd_array in core #40319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 10 commits into from
Apr 2, 2021
Merged

STYLE use pd_array in core #40319

merged 10 commits into from
Apr 2, 2021

Conversation

MarcoGorelli
Copy link
Member

As requested by @jbrockmendel here

@MarcoGorelli MarcoGorelli added the Code Style Code style, linting, code_checks label Mar 9, 2021
@MarcoGorelli MarcoGorelli force-pushed the pd_array-in-core branch 2 times, most recently from bcc88bc to 192dcdc Compare March 9, 2021 11:39
@jbrockmendel
Copy link
Member

@MarcoGorelli i think an easier solution is

construction.py:

def pd_array(...):

__init__.py:

from pandas.core.construction import pd_array as array

@MarcoGorelli MarcoGorelli requested review from jbrockmendel and removed request for jbrockmendel March 10, 2021 09:49
jorisvandenbossche
jorisvandenbossche requested changes Mar 10, 2021
View reviewed changes
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to have a contradictory opinion .. but I am personally think that using the import alias where it's used is nicer. I would prefer to keep the actual name of the function (array) the same as the name how it is exposed publicly, since you still see this name (eg in the repr of the function object), and it's only distracting if that's different IMO.

@jorisvandenbossche
Copy link
Member

For me this is fine (but it's @jbrockmendel that suggested otherwise)

The linting error seems relevant.

@jbrockmendel
Copy link
Member

joris makes a reasonable point about repr(pd_array). im fine with the approach in this PR

@MarcoGorelli
Copy link
Member Author

The linting error seems relevant.

Indeed - fixed, thanks!

@jorisvandenbossche jorisvandenbossche added this to the 1.3 milestone Mar 15, 2021
@jorisvandenbossche
Copy link
Member

@MarcoGorelli can you do another (final) merge of master to be sure there are no new failures?

@jreback
Copy link
Contributor

jreback commented Mar 30, 2021

lgtm. @MarcoGorelli can you merge master and ping on greenish

@jreback
Copy link
Contributor

jreback commented Apr 2, 2021

@MarcoGorelli if you can merge master once more

@jreback jreback merged commit fb04a42 into pandas-dev:master Apr 2, 2021
@jreback
Copy link
Contributor

jreback commented Apr 2, 2021

thanks @MarcoGorelli

@MarcoGorelli MarcoGorelli deleted the pd_array-in-core branch April 2, 2021 16:29
vladu pushed a commit to vladu/pandas that referenced this pull request Apr 5, 2021
@MarcoGorelli @vladu
STYLE use pd_array in core (pandas-dev#40319)
2f4755e
JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
@MarcoGorelli @JulianWgs
STYLE use pd_array in core (pandas-dev#40319)
25303a8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jreback jreback jreback approved these changes

@ShaharNaveh ShaharNaveh Awaiting requested review from ShaharNaveh

@jbrockmendel jbrockmendel Awaiting requested review from jbrockmendel

@jorisvandenbossche jorisvandenbossche Awaiting requested review from jorisvandenbossche

Assignees
No one assigned
Labels
Code Style Code style, linting, code_checks
Projects
None yet
Milestone
1.3
Development

Successfully merging this pull request may close these issues.
4 participants
@MarcoGorelli @jbrockmendel @jorisvandenbossche @jreback